fix: remove duplicate line in known_hosts when minikube deletes #16965

ComradeProgrammer · 2023-07-31T02:44:01Z

FIX #16868
This PR remove the public key of the node from user's known_hosts file when executing minikube delete --all

What it does:

when we run minikube ssh-host --append-known for a node, the inserted key is not only written to the known_hosts file, but also written to $MINIHOME/machines/{node name}/known_host
when we run minikube delete --all, it go through all the folders under this $MINIHOME/machines/, and remove keys stored in each folder from known_hosts file
step 2 is run in a background goroutine so that it won't block the minikube delete for a long time

Before
Nothing happens when minikube delete, and old items in known_hosts remain

After
In a minikube cluster with multiple nodes

$ minikube node list
minikube	192.168.49.2
minikube-m02	192.168.49.3

execute minikube ssh-host --append-known for a node

$ minikube ssh-host -n minikube-m02 --append-known
Host added: /Users/tjm/.ssh/known_hosts ([127.0.0.1]:55943)

when the public key appears in known_hosts file

$ cat ~/.ssh/known_hosts
github.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl
github.com ssh-rsa AAAAB3N...
[127.0.0.1]:55943 ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDScW6Jub3Jh5UYtp9jV9GsmqHZB8SEaTYWw+Hmm8T0IU0y/IMGVw3qdQ4G8xBVQu05icRn6CXI0h+cc5kbVC4WkvObAAcCzTq1KYkgBHiKunCWZkOCKIdOGxN5sZBOLS9GrMON8BfviKpQHNP9t4ptmqKsn9s55acS58oW2YtMEUyEHzNTaBJwjXNewucsOd9LJJ85qf04krmYFSOw6qldTIdK3bATntdsbWhE4BZR3SbBM143AVdY9bjFNsJEcC8/u5zPMojURkaKGMV2pz61BBoLjYG8EPvLGwSyb4Hvd27Hyk4R5YkYbPo4Xhvp2AtbXcR1RapJxg29xRa6WzksFrjMhuaAmCky3HzwUAGTEUKaaeW6gh9c3wj/SHTjzppXTIzTMci2C4Dsi5EeZwxXx5hwgbguvBRZF0eyRu3nJm7iECSFOmkr5esLi4aYOVdgyzfnDFUc2AdvX2tN3VUs+nl29qlvYfoXPWeAyGVJC/V7sOXtsbm+Ieuq1pbPQS0=

it also appears in $MINIHOME/machines/{node name}/known_host

$ cat ~/.minikube/machines/minikube-m02/known_host
[127.0.0.1]:55943 ssh-rsa AAAAB3NzaC1yc2EAAAADAQABAAABgQDScW6Jub3Jh5UYtp9jV9GsmqHZB8SEaTYWw+Hmm8T0IU0y/IMGVw3qdQ4G8xBVQu05icRn6CXI0h+cc5kbVC4WkvObAAcCzTq1KYkgBHiKunCWZkOCKIdOGxN5sZBOLS9GrMON8BfviKpQHNP9t4ptmqKsn9s55acS58oW2YtMEUyEHzNTaBJwjXNewucsOd9LJJ85qf04krmYFSOw6qldTIdK3bATntdsbWhE4BZR3SbBM143AVdY9bjFNsJEcC8/u5zPMojURkaKGMV2pz61BBoLjYG8EPvLGwSyb4Hvd27Hyk4R5YkYbPo4Xhvp2AtbXcR1RapJxg29xRa6WzksFrjMhuaAmCky3HzwUAGTEUKaaeW6gh9c3wj/SHTjzppXTIzTMci2C4Dsi5EeZwxXx5hwgbguvBRZF0eyRu3nJm7iECSFOmkr5esLi4aYOVdgyzfnDFUc2AdvX2tN3VUs+nl29qlvYfoXPWeAyGVJC/V7sOXtsbm+Ieuq1pbPQS0=

now after running minikube delete --all, the deprecated public key in known_hosts file also disappears

$ ./minikube delete --all
🔥  Deleting "minikube" in docker ...
🔥  Removing /Users/tjm/.minikube/machines/minikube ...
🔥  Removing /Users/tjm/.minikube/machines/minikube-m02 ...
💀  Removed all traces of the "minikube" cluster.
🔥  Successfully deleted all profiles

$ cat ~/.ssh/known_hosts
github.com ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIOMqqnkVzrm0SdG6UOoqKLsabgH5C9okWi0dh2l9GKJl
github.com ssh-rsa AAAAB3N...

linux-foundation-easycla · 2023-07-31T02:44:04Z

The committers listed above are authorized under a signed CLA.

✅ login: ComradeProgrammer / name: 錦南路之花 (ffd458c, c22893e, baccf08)

k8s-ci-robot · 2023-07-31T02:44:10Z

Hi @ComradeProgrammer. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

minikube-bot · 2023-07-31T02:48:05Z

Can one of the admins verify this patch?

cmd/minikube/cmd/delete.go

medyagh · 2023-07-31T18:23:52Z

❌ The email address for the commit (0835970) is not linked to the GitHub account, preventing the EasyCLA check. Consult this Help Article and GitHub Help to resolve. (To view the commit's email address, add .patch at the end of this PR page's URL.) For further assistance with EasyCLA, please submit a support request ticket.

you might need to rebase and ensure you are logged in

spowelljr

This only deletes the known_hosts entry on minikube delete --all we should also delete them for a non --all delete as well.

medyagh · 2024-07-25T17:24:19Z

@ComradeProgrammer plz take another look so we could merge this before the release in upcoming weeks :)

medyagh · 2024-07-25T17:37:57Z

/ok-to-test

medyagh · 2024-08-21T18:42:29Z

@ComradeProgrammer have you addressed the comments from steven ? can you plz take another look so we could fit in in the release

ComradeProgrammer · 2024-08-30T20:55:48Z

@spowelljr @medyagh updated

pkg/util/utils.go

k8s-ci-robot · 2024-09-13T17:24:35Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: ComradeProgrammer
Once this PR has been reviewed and has the lgtm label, please assign prezha for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

cmd/minikube/cmd/delete.go

Co-authored-by: Steven Powell <[email protected]>

cmd/minikube/cmd/delete.go

pkg/util/utils_test.go

spowelljr · 2024-10-02T20:26:23Z

cmd/minikube/cmd/ssh-host.go

+		// these keys can be removed properly
+		_, cc := mustload.Partial(ClusterFlagValue())
+		knownHostPath := filepath.Join(localpath.MiniPath(), "machines", config.MachineName(*cc, *n), "known_host")
+		if err := os.WriteFile(knownHostPath, []byte(keys), 0666); err != nil {


Any reason this needs to be 666 vs 644?

minikube-pr-bot · 2024-10-02T20:45:52Z

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 16965) |
+----------------+----------+---------------------+
| minikube start | 48.3s    | 48.7s               |
| enable ingress | 27.0s    | 15.3s               |
+----------------+----------+---------------------+

Times for minikube start: 47.0s 48.0s 47.8s 51.3s 47.5s
Times for minikube (PR 16965) start: 46.2s 50.0s 49.3s 49.1s 48.7s

Times for minikube ingress: 76.0s 15.0s 14.5s 14.5s 15.0s
Times for minikube (PR 16965) ingress: 15.0s 15.5s 15.5s 14.5s 15.9s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 16965) |
+----------------+----------+---------------------+
| minikube start | 22.3s    | 22.7s               |
| enable ingress | 13.0s    | 13.2s               |
+----------------+----------+---------------------+

Times for minikube start: 23.6s 23.1s 21.0s 23.1s 20.4s
Times for minikube (PR 16965) start: 24.6s 21.3s 20.6s 23.0s 24.2s

Times for minikube ingress: 12.8s 12.8s 12.3s 13.3s 13.9s
Times for minikube (PR 16965) ingress: 12.3s 13.3s 12.8s 13.8s 13.8s

docker driver with containerd runtime

+-------------------+----------+---------------------+
|      COMMAND      | MINIKUBE | MINIKUBE (PR 16965) |
+-------------------+----------+---------------------+
| minikube start    | 21.3s    | 21.0s               |
| ⚠️  enable ingress | 23.1s    | 28.5s ⚠️             |
+-------------------+----------+---------------------+

Times for minikube start: 22.3s 19.2s 19.2s 22.9s 22.9s
Times for minikube (PR 16965) start: 23.1s 20.2s 19.2s 22.3s 20.5s

Times for minikube ingress: 22.8s 23.3s 22.8s 22.8s 23.8s
Times for minikube (PR 16965) ingress: 22.8s 40.3s 31.3s 24.3s 23.8s

minikube-pr-bot · 2024-10-02T23:37:53Z

Here are the number of top 10 failed tests in each environments with lowest flake rate.

Environment	Test Name	Flake Rate
Docker_Linux_docker_arm64 (1 failed)	TestAddons/serial/GCPAuth/PullSecret(gopogh)	Unknown
Docker_Cloud_Shell (7 failed)	TestAddons/serial/GCPAuth/PullSecret(gopogh)	Unknown
Docker_Cloud_Shell (7 failed)	TestAddons/serial/Volcano(gopogh)	1.82% (chart)
KVM_Linux (1 failed)	TestAddons/serial/GCPAuth/PullSecret(gopogh)	Unknown
KVM_Linux_containerd (1 failed)	TestFunctional/parallel/MountCmd/specific-port(gopogh)	2.29% (chart)
Docker_Linux (2 failed)	TestAddons/serial/GCPAuth/PullSecret(gopogh)	Unknown
Docker_Linux (2 failed)	TestKubernetesUpgrade(gopogh)	1.70% (chart)
none_Linux (1 failed)	TestAddons/serial/GCPAuth/PullSecret(gopogh)	Unknown
Docker_Linux_crio_arm64 (4 failed)	TestAddons/serial/GCPAuth/PullSecret(gopogh)	Unknown
Docker_Linux_crio_arm64 (4 failed)	TestMultiControlPlane/serial/DeleteSecondaryNode(gopogh)	5.68% (chart)
Docker_Linux_crio (3 failed)	TestAddons/serial/GCPAuth/PullSecret(gopogh)	Unknown

Besides the following environments also have failed tests:

KVM_Linux_crio: 35 failed (gopogh)
Hyperkit_macOS: 11 failed (gopogh)
QEMU_macOS: 100 failed (gopogh)
Docker_Linux_containerd_arm64: 2 failed (gopogh)

To see the flake rates of all tests by environment, click here.

medyagh · 2024-10-16T18:52:50Z

we need to test it manually.... @ComradeProgrammer can we do a few Manual Scenarios and paste them here, on both linux and macos maybe? or even windows
this is something hard to test

Co-authored-by: Steven Powell <[email protected]>

k8s-ci-robot · 2024-10-30T15:35:20Z

@ComradeProgrammer: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
pull-minikube-build	`c22893e`	link	true	`/test pull-minikube-build`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Jul 31, 2023

k8s-ci-robot requested review from afbjorklund and spowelljr July 31, 2023 02:44

k8s-ci-robot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Jul 31, 2023

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 31, 2023

medyagh requested changes Jul 31, 2023

View reviewed changes

cmd/minikube/cmd/delete.go Outdated Show resolved Hide resolved

medyagh reviewed Jul 31, 2023

View reviewed changes

cmd/minikube/cmd/delete.go Outdated Show resolved Hide resolved

ComradeProgrammer force-pushed the clear_hosts branch from f60898c to 531365c Compare August 10, 2023 14:16

ComradeProgrammer force-pushed the clear_hosts branch from 531365c to b2fcf09 Compare August 14, 2023 17:53

ComradeProgrammer force-pushed the clear_hosts branch from b2fcf09 to 395980b Compare August 14, 2023 17:56

ComradeProgrammer force-pushed the clear_hosts branch from 395980b to 9bbab0f Compare August 14, 2023 18:06

spowelljr requested changes Jul 22, 2024

View reviewed changes

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Jul 25, 2024

This comment has been minimized.

Sign in to view

ComradeProgrammer force-pushed the clear_hosts branch from 55d2c6e to 6184085 Compare August 30, 2024 20:53

ComradeProgrammer requested a review from spowelljr August 30, 2024 20:55

This comment has been minimized.

Sign in to view

medyagh requested changes Sep 13, 2024

View reviewed changes

pkg/util/utils.go Show resolved Hide resolved

k8s-ci-robot removed the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 13, 2024

ComradeProgrammer force-pushed the clear_hosts branch from 6184085 to 2d65564 Compare September 22, 2024 18:56

This comment has been minimized.

Sign in to view

spowelljr requested changes Oct 1, 2024

View reviewed changes

fix: remove duplicate line in known_hosts when minikube deletes

baccf08

ComradeProgrammer force-pushed the clear_hosts branch from 2d65564 to baccf08 Compare October 2, 2024 19:54

Apply suggestions from code review

ffd458c

Co-authored-by: Steven Powell <[email protected]>

spowelljr requested changes Oct 2, 2024

View reviewed changes

Apply suggestions from code review

c22893e

Co-authored-by: Steven Powell <[email protected]>

ComradeProgrammer force-pushed the clear_hosts branch from 4d12e4b to c22893e Compare October 30, 2024 15:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: remove duplicate line in known_hosts when minikube deletes #16965

fix: remove duplicate line in known_hosts when minikube deletes #16965

ComradeProgrammer commented Jul 31, 2023 •

edited

Loading

linux-foundation-easycla bot commented Jul 31, 2023 •

edited

Loading

k8s-ci-robot commented Jul 31, 2023

minikube-bot commented Jul 31, 2023

medyagh commented Jul 31, 2023

spowelljr left a comment

medyagh commented Jul 25, 2024

medyagh commented Jul 25, 2024

This comment has been minimized.

This comment has been minimized.

medyagh commented Aug 21, 2024

ComradeProgrammer commented Aug 30, 2024

This comment has been minimized.

This comment has been minimized.

k8s-ci-robot commented Sep 13, 2024

This comment has been minimized.

This comment has been minimized.

spowelljr Oct 2, 2024

minikube-pr-bot commented Oct 2, 2024

minikube-pr-bot commented Oct 2, 2024

medyagh commented Oct 16, 2024

k8s-ci-robot commented Oct 30, 2024

fix: remove duplicate line in known_hosts when minikube deletes #16965

Are you sure you want to change the base?

fix: remove duplicate line in known_hosts when minikube deletes #16965

Conversation

ComradeProgrammer commented Jul 31, 2023 • edited Loading

linux-foundation-easycla bot commented Jul 31, 2023 • edited Loading

k8s-ci-robot commented Jul 31, 2023

minikube-bot commented Jul 31, 2023

medyagh commented Jul 31, 2023

spowelljr left a comment

Choose a reason for hiding this comment

medyagh commented Jul 25, 2024

medyagh commented Jul 25, 2024

This comment has been minimized.

This comment has been minimized.

medyagh commented Aug 21, 2024

ComradeProgrammer commented Aug 30, 2024

This comment has been minimized.

This comment has been minimized.

k8s-ci-robot commented Sep 13, 2024

This comment has been minimized.

This comment has been minimized.

spowelljr Oct 2, 2024

Choose a reason for hiding this comment

minikube-pr-bot commented Oct 2, 2024

minikube-pr-bot commented Oct 2, 2024

medyagh commented Oct 16, 2024

k8s-ci-robot commented Oct 30, 2024

ComradeProgrammer commented Jul 31, 2023 •

edited

Loading

linux-foundation-easycla bot commented Jul 31, 2023 •

edited

Loading